Skip to content

Generate C enums from internal enums, introduce Z_PARAM_ENUM() #20917

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
arnaud-lb wants to merge 31 commits into
base: master
Choose a base branch
from
Open

Generate C enums from internal enums, introduce Z_PARAM_ENUM() #20917

arnaud-lb wants to merge 31 commits into from

Conversation

@arnaud-lb
Copy link
Member

@arnaud-lb arnaud-lb commented Jan 12, 2026 •
edited
Loading

Update gen_stubs.php to generate C enums from internal enums. Enum values can be compared to the result of zend_enum_fetch_case_id(zend_object*).

The generated enums are added to separate files named {$extensionName}_decl.h (one for each extension declaring some enums), so that it's possible to include these from anywhere. _arginfo.h files would generate warnings if we tried to include them in a compilation unit that doesn't call the register_{$class} functions, for instance.

Introduce Z_PARAM_ENUM() (similarly to #20898).

cc @TimWolla

UPGRADING.INTERNALS:

diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS
index f23a610b849..0f591575a97 100644
--- a/UPGRADING.INTERNALS
+++ b/UPGRADING.INTERNALS
@@ -56,11 +56,20 @@ PHP 8.6 INTERNALS UPGRADE NOTES
     node.
   . The zend_exception_save() and zend_exception_restore() functions were
     removed.
+  . The php_math_round_mode_from_enum() function now takes a
+    zend_enum_RoundingMode parameter.
+  . Added Z_PARAM_ENUM().
+  . Added zend_enum_fetch_case_id().
 
 ========================
 2. Build system changes
 ========================
 
+  . build/gen_stub.php may now generate a _decl.h file in addition to
+    the _arginfo.h file, if the stub declares enums. For each enum, the file
+    will contain a C enum. Enum values can be compared to the result of
+    zend_enum_fetch_case_id(zend_object*).
+
 ========================
 3. Module changes
 ========================

TimWolla
TimWolla reviewed Jan 12, 2026
View reviewed changes
Copy link
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some first comments. Will take another look once rebased after the merge of #20915.

Zend/zend_enum.h

static zend_always_inline zend_long zend_enum_fetch_case_id(zend_object *zobj)
{
ZEND_ASSERT(zobj->ce->ce_flags & ZEND_ACC_ENUM);
Copy link
Member

@TimWolla TimWolla Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assert is redundant with the one in zend_enum_obj_from_obj(). I also don't see how it could help with codegen.

Copy link
Member Author

@arnaud-lb arnaud-lb Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I though about this when adding this assert, but if we consider functions as opaque APIs, we don't know that zend_enum_obj_from_obj() has redundant asserts.

Copy link
Member

@TimWolla TimWolla Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is fair, but in this case it is probably reasonable to expect zend_enum_obj_from_obj() to do the verification (if necessary), since that's the purpose of the function, especially since a failed assert is always a programmer error.

Copy link
Member

@TimWolla TimWolla Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No particularly strong feelings either way, though.

@arnaud-lb arnaud-lb force-pushed the enum-c-enum branch 2 times, most recently from 0b7ca66 to adfa289 Compare January 13, 2026 12:24
@iluuu1994
Copy link
Member

iluuu1994 commented Jan 13, 2026

Nice approach. How will ADTs be handled? #define Z_PARAM_ENUM_EX(dest, obj, _ce) comes to mind. That should probably work.

@arnaud-lb
Copy link
Member Author

arnaud-lb commented Jan 13, 2026

@iluuu1994 yes this should work. Alternatively it's possible to use Z_PARAM_OBJ_OF_CLASS() + zend_enum_fetch_case_id().

@arnaud-lb arnaud-lb marked this pull request as ready for review January 13, 2026 13:56
TimWolla
TimWolla reviewed Jan 13, 2026
View reviewed changes
@TimWolla TimWolla self-requested a review January 13, 2026 16:14
Girgias
Girgias reviewed Jan 13, 2026
View reviewed changes
DanielEScherzer
DanielEScherzer reviewed Jan 13, 2026
View reviewed changes
@TimWolla TimWolla requested a review from kocsismate January 31, 2026 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TimWolla TimWolla TimWolla approved these changes

@Girgias Girgias Girgias left review comments

@devnexen devnexen devnexen approved these changes

@ndossche ndossche ndossche approved these changes

@DanielEScherzer DanielEScherzer DanielEScherzer approved these changes

@dstogov dstogov Awaiting requested review from dstogov dstogov is a code owner

@bukka bukka Awaiting requested review from bukka bukka is a code owner

@zeriyoshi zeriyoshi Awaiting requested review from zeriyoshi zeriyoshi is a code owner

@SakiTakamachi SakiTakamachi Awaiting requested review from SakiTakamachi SakiTakamachi is a code owner

@kocsismate kocsismate Awaiting requested review from kocsismate kocsismate is a code owner

+1 more reviewer

@thg2k thg2k thg2k left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

ABI break Category: Build System Category: Engine Extension: bcmath Extension: dom Extension: pcntl Extension: random Extension: reflection Extension: standard Extension: uri Extension: zend_test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@arnaud-lb @iluuu1994 @TimWolla @thg2k @devnexen @kocsismate @ndossche @Girgias @DanielEScherzer